Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/issue 189 #190

Merged
merged 5 commits into from
Sep 12, 2023
Merged

Feature/issue 189 #190

merged 5 commits into from
Sep 12, 2023

Conversation

sliu008
Copy link
Contributor

@sliu008 sliu008 commented Aug 31, 2023

Github Issue: #189 (replace NUM with Github issue number)

Description

Fix temporal subsetting for SWOT data

Overview of work done

  • Swot data was failing to open using xarray cause of an overflow error in time variable
  • Add function to test opening of granule file to determine if we need to use mask_and_scale argument
  • Modify encoding when writing file to use original file encoding

Overview of verification done

  • Tested locally harmony call to make sure swot data is subsetted and data looks like original (need verify with Jack)
  • Made sure all tests still pass with changes

Overview of integration done

  • Tested on local harmony with swot harmony call

PR checklist:

  • Linted
  • Updated unit tests
  • Updated changelog
  • Integration testing

See Pull Request Review Checklist for pointers on reviewing this pull request


# Check for the specific OverflowError message
if "Python int too large to convert to C long" in traceback_str and "Failed to decode variable 'time': unable to decode time units" in traceback_str:
args["mask_and_scale"] = True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By setting mask_and_scale to true, the resulting granule will have no scale/offset attributes because those will be applied to the data variables. Ideally we'd maintain the old behavior (where the subsetted granule has the same scale/offset attributes) -- would such a thing be possible? The hard way to do this would be to re-apply the scale/offset to each data variable, but maybe there's an easier way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct, im not sure if we can undo this, i haven't found any other way where i can open the file using xarray without the mask_and_scale options

traceback_str = traceback.format_exc()

# Check for the specific OverflowError message
if "Python int too large to convert to C long" in traceback_str and "Failed to decode variable 'time': unable to decode time units" in traceback_str:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded 'time' variable name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have the intention to only catch the "Python int too large to convert to C long" for the time variable, although i can change it catch for any.

@jamesfwood jamesfwood merged commit 6b9c957 into develop Sep 12, 2023
3 checks passed
@jamesfwood jamesfwood deleted the feature/issue-189 branch September 12, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants